-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create detection LLM endpoint #232
base: 223-creating-activity-dashboard
Are you sure you want to change the base?
Create detection LLM endpoint #232
Conversation
ebe5fe7
to
a91ac59
Compare
79c3666
to
848f07e
Compare
51434ac
to
a4073a0
Compare
92b2879
to
372ac51
Compare
da0708f
to
a6678bc
Compare
72b78fa
to
98d9c8e
Compare
fb9e0b5
to
5b25cbf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are some review comments. Since the migration is now split over two PRs maybe it makes sense to just merge the stacked PRs together after you have implemented the review comments and then we just merge jointly for the production deployment.
@@ -46,7 +48,7 @@ public ActivityDTO getActivity(String login) { | |||
pullRequestBadPracticeRepository.findAssignedByLoginAndOpen(login); | |||
|
|||
Map<PullRequest, List<PullRequestBadPracticeDTO>> pullRequestBadPracticesMap = openPulLRequestBadPractices | |||
.stream() | |||
.stream().filter(pullRequestBadPractice -> !pullRequestBadPractice.isResolved()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that we are only showing the unresolved detected bad practices? So the resolved ones disappear from the UI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but in the follow up Pr I adjusted the detected logic and there also show resolved bad practices
...on-server/src/main/java/de/tum/in/www1/hephaestus/activity/model/PullRequestBadPractice.java
Outdated
Show resolved
Hide resolved
...a/de/tum/in/www1/hephaestus/activity/badpracticedetector/PullRequestBadPracticeDetector.java
Outdated
Show resolved
Hide resolved
...a/de/tum/in/www1/hephaestus/activity/badpracticedetector/PullRequestBadPracticeDetector.java
Show resolved
Hide resolved
...ication-server/src/main/java/de/tum/in/www1/hephaestus/config/BadPracticeDetectorConfig.java
Outdated
Show resolved
Hide resolved
@@ -1,8 +1,8 @@ | |||
<?xml version="1.1" encoding="UTF-8" standalone="no"?> | |||
<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog" xmlns:ext="http://www.liquibase.org/xml/ns/dbchangelog-ext" xmlns:pro="http://www.liquibase.org/xml/ns/pro" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog-ext http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-ext.xsd http://www.liquibase.org/xml/ns/pro http://www.liquibase.org/xml/ns/pro/liquibase-pro-latest.xsd http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd"> | |||
<changeSet author="iam-flo" id="1738063112622-1"> | |||
<createTable tableName="pull_request_bad_practice"> | |||
<column name="id" type="BIGINT"> | |||
<createTable tableName="pullrequestbadpractice"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure about this migration? I think if this builds on the previous migration it will not work as intended. You are never deleting the pull_request_bad_practice
table and migrating its data. Maybe you also don't need to rename it in the first place.
Also is the @NonNull
that you added considered?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i agree, we should merge this first and then i will rework this migration
server/intelligence-service/app/detector/bad_practice_detector.py
Outdated
Show resolved
Hide resolved
server/intelligence-service/app/detector/bad_practice_detector.py
Outdated
Show resolved
Hide resolved
server/intelligence-service/app/detector/bad_practice_detector.py
Outdated
Show resolved
Hide resolved
@@ -58,7 +58,7 @@ | |||
<brn-collapsible-content> | |||
@for (badpractice of badPractices(); track badpractice.title) { | |||
<brn-separator hlmSeparator /> | |||
<app-bad-practice-card [title]="badpractice.title" [description]="badpractice.description" [resolved]="false"></app-bad-practice-card> | |||
<app-bad-practice-card [title]="badpractice.title" [description]="badpractice.description" [resolved]="badpractice.resolved"></app-bad-practice-card> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you can potentially also just pass the DTO type as input so you don't need to do each field separately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is done similarly in the other components. what is the best practice?
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
144efa3
to
0951d43
Compare
Motivation
Fixes #123. Enables detection of bad PR description practices based on predefined rules in the application server.
Description
Sends PR description and rules to LLM for evaluation.
Configures LLM to analyze descriptions and detect bad practices.